Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NSFS | NC | Add Schema Validation to Bucket and Account Add and Update (NC NSFS CLI) #7702

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

shirady
Copy link
Contributor

@shirady shirady commented Jan 7, 2024

Explain the changes

  1. Move the NC NSFS schema validation that we had in file src/sdk/bucketspace_fs.js (from PR NC | NSFS | Schema validation #7544) to a separate file (src/manage_nsfs/nsfs_schema_utils.js) to reuse it.
  2. Edit the values of s3_policy and fs_backend to be undefined (and not empty '', as it passed as an argument).
  3. Validate the data using schema and return an error in case it doesn't match (details would be passed according to the original message), using a mapping ManageCLIError.RPC_ERROR_TO_MANAGE.
  4. Update the unit tests for the cases of s3_policy and fs_backend deletions (separate the argument that we pass as '' to the actual property that would be undefined).
  5. Change the uid and gid extract in function fetch_account_data to be undefined when the argument user was passed (and not false).
  6. Use schema_utils.strictify with option additionalProperties: false in the nsfs_account_schema and nsfs_bucket_schema.
  7. Add the versioning enum to the bucket schema.
  8. Add tests for the schema validation of nsfs_account_schema and nsfs_bucket_schema.

Issues: Fixed #7694

  1. Add schema validation to bucket and account add and update - using NC NSFS CLI (fs_backend changes in a separate PR).
  2. GAP - I couldn't ref bucket_api schema and had to duplicate the values, else got the error: Error: reference "bucket_api" resolves to more than one schema in the CI in job run-unit-tests.

Testing Instructions:

  1. For tests that were edited, please run:
    sudo node ./node_modules/.bin/_mocha src/test/unit_tests/test_nc_nsfs_cli.js
    sudo node ./node_modules/.bin/_mocha src/test/unit_tests/test_bucketspace_fs.js
  2. For new tests, please run:
    npx jest test_nc_nsfs_schema_validation.test.js
    (no need for sudo permission, can see results live also with jest extension in VSCode).
  • Doc added/updated
  • Tests added
  • Tests edited (see comment above)

@shirady
Copy link
Contributor Author

shirady commented Jan 7, 2024

Manual Testing Instructions:

Schema validation for buckets and accounts

First, create the FS_ROOT and give permissions:

mkdir -p /tmp/nsfs_root1
chmod 777 /tmp/nsfs_root1

This will be the argument for:

  • new_buckets_path flag that we will use in the account commands
  • path in the buckets commands.

In each case the commands work, hence we added the code change (to sabotage the created object and make sure that it creates an error, in each case we chose one of the properties that is required to be deleted.
for example in account add we would add this line:
data.email = undefined; // test
before passing to the schema validation, and see the error:
{
"error": {
"code": "InvalidSchema",
"message": "Schema invalid, please use required properties",
"detail": "must have required property 'email'"
}

Account add

  1. Add account sudo node src/cmd/manage_nsfs account add --name <account-name> --email <account-email> --new_buckets_path /tmp/nsfs_root1 --uid <uid-number> --gid <gid-number>
  2. Add deletion line of email.

Account update

  1. Create an account (the account that we would update).
  2. Update account sudo node src/cmd/manage_nsfs account update --name <account-name> --email <change-the-account-email>
  3.  Add deletion line of email (in the first condition - it's neither a name update nor an access keys update).

Bucket add

  1. Create an account (the account that we would create the bucket for it).
  2. Add bucket sudo node src/cmd/manage_nsfs bucket add --name <bucket-name> --email <account-email> --path /tmp/nsfs_root1
  3. Add deletion line of creation_date.

Bucket update

  1. Create a bucket (the bucket that we would update).
  2. Update bucket sudo node src/cmd/manage_nsfs account update --name <account-name> --email <change-the-account-email>
  3. Add deletion line of creation_date (in the first condition - it's not a name update).

Remove bucket policy and FS backend:

First, Create bucket sudo node src/cmd/manage_nsfs bucket add --name <bucket-name> --email <account-email> --path /tmp/nsfs_root1 In the next commands please use the same <bucket-name> as you created here.

Bucket policy:

  1. Add bucket policy: sudo node src/cmd/manage_nsfs bucket update --name <bucket-name> --bucket_policy '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"AWS":["*"]},"Action":["s3:*"],"Resource":["arn:aws:s3:::<bucket-name>/*","arn:aws:s3:::<bucket-name>"]}]}' (replace the <bucket-name> in the bucket policy json argument as well)
  2. Remove bucket policy: sudo node src/cmd/manage_nsfs bucket update --name <same-name-above> --bucket_policy ''

FS backend

  1. Add FS backend: sudo node src/cmd/manage_nsfs bucket update --name <bucket-name> --fs_backend GPFS
  2. Remove FS backend: sudo node src/cmd/manage_nsfs bucket update --name <bucket-name> --fs_backend ''

src/api/common_api.js Outdated Show resolved Hide resolved
src/server/utils/nsfs_schema_utils.js Outdated Show resolved Hide resolved
src/server/object_services/schemas/nsfs_bucket_schema.js Outdated Show resolved Hide resolved
src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
@romayalon
Copy link
Contributor

@shirady please add automatic tests

src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
@shirady shirady force-pushed the schema-validation branch 8 times, most recently from 144b264 to f9a310e Compare January 8, 2024 13:53
@shirady shirady requested a review from romayalon January 8, 2024 14:33
@shirady shirady force-pushed the schema-validation branch from a0cf8d5 to 1223079 Compare January 9, 2024 08:22
src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
@shirady shirady force-pushed the schema-validation branch 3 times, most recently from 8e53ebc to c97c3cb Compare January 9, 2024 09:59
@shirady shirady requested a review from romayalon January 9, 2024 10:31
Copy link
Contributor

@romayalon romayalon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, as mentioned on #7694, bucketspace_fs missing validations will be added in the next PR

@shirady shirady force-pushed the schema-validation branch from c97c3cb to 2940536 Compare January 9, 2024 11:24
@shirady shirady force-pushed the schema-validation branch 2 times, most recently from 3129b48 to 374db01 Compare January 9, 2024 11:33
@shirady shirady changed the title NSFS | NC | Add Schema Validation to Bucket and Account Add and Update NSFS | NC | Add Schema Validation to Bucket and Account Add and Update (NC NSFS CLI) Jan 9, 2024
@shirady shirady force-pushed the schema-validation branch 2 times, most recently from d326558 to ae7a132 Compare January 9, 2024 13:23
1. Move the NC NSFS schema validation that we had in file `src/sdk/bucketspace_fs.js` (from PR noobaa#7544) to a separate file (`src/manage_nsfs/nsfs_schema_utils.js`) to reuse it.
2. Edit the values of `s3_policy` and `fs_backend` to be undefined (and not empty `''`, as it passed as an argument).
3. Validate the data using schema and return an error in case it doesn't match (details would be passed according to the original message), using a mapping `ManageCLIError.RPC_ERROR_TO_MANAGE`.
4. Update the unit tests for the cases of `s3_policy` and `fs_backend` deletions (separate the argument that we pass as `''` to the actual property that would be `undefined`).
5. Change the `uid` and `gid` extract in function `fetch_account_data` to be `undefined` when the argument user was passed (and not `false`).
6. Use `schema_utils.strictify` with option  `additionalProperties: false` in the `nsfs_account_schema` and `nsfs_bucket_schema`.
7. Add the `versioning` enum to the bucket schema.
8. Add tests for the schema validation of nsfs_account_schema and nsfs_bucket_schema.

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
@shirady shirady merged commit 5eac6a9 into noobaa:master Jan 10, 2024
9 checks passed
@shirady shirady deleted the schema-validation branch January 16, 2024 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NSFS | NC | AJV bucket and account schemas should be validated on creation
3 participants